Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor deprecated antd Dropdown menus #6898

Merged
merged 39 commits into from
Mar 23, 2023
Merged

Refactor deprecated antd Dropdown menus #6898

merged 39 commits into from
Mar 23, 2023

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Mar 3, 2023

Antd 4.2x introduced deprecation warnings for <Menu>components within <Dropdown> elements. This PR replaces the React <Menu> components with heir object equivalents.

Most changes/files are rather straight forward conversions. Only the annotations context menu is more involved since is is created from a million different function, components, etc. Hopefully the strict typing of all menu items prevents any new bugs.

I tested all menus and clicked on all items but I might have missed some by mistake...

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Test that all the menu still show (or at least the most important menus)
  • Consider performance. I tested with an annotation with 1700+ trees and the Trees tab remained smooth. Here is the NML file
    detection_no_size_threshold.nml.zip
  • Randomly sample some menu entries for testing

Issues:


(Please delete unneeded items, merge only when none are left open)

…bar-menu

* 'master' of github.com:scalableminds/webknossos:
  Fix exploring datasets via http (without s) (#6863)
…bar-menu

* 'master' of github.com:scalableminds/webknossos:
  Fix style of connectome tab by restricting directory tree style (#6864)
  Remove legacy datasets tab and use new compact route (#6834)
  Fix basicauth for exploring remote http datasets (#6866)
…bar-menu

* 'master' of github.com:scalableminds/webknossos:
  Add functions to get and set segment colors to the frontend API (#6853)
  Ensure that segments are added to segment list when using quick select... (#6867)
…d-v4.24

* 'master' of github.com:scalableminds/webknossos:
  Update deprecated antd <Menu> (#6860)
…d-v4.24

* 'master' of github.com:scalableminds/webknossos:
  Implement http range requests for HttpsSeekableByteChannel (#6869)
  new GH action for adding issues to project board
  Fix links in Changelog (#6881)
  adds dedicated explore method for zarr datasets with a datasource-properties.json (#6879)
  Release 23.03.0 (#6880)
  Fix superUser being wrongly marked as organization owners (#6876)
  Followups for OME-TIFF export (#6874)
  Fix reload-precomputed-mesh functionality (#6875)
  Adds OME-TIFF export (#6838)
  Add evolutions 99,100 to migration guide (#6871)
  Add link to imprint and privacy to help menu (#6870)
  Annotation Locking Mechanism (#6819)
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Upgrade Antd to v4.24 (#6865)
  Support n5 end-chunks with chunksize differing from metadata chunksize (#6890)
@@ -186,7 +186,7 @@ export function FolderTreeSidebar({
/>
</div>
<Dropdown
overlay={createMenu}
menu={createMenu()}
placement="bottom"
// The overlay is generated lazily. By default, this would make the overlay
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mh, the menu prop no longer supports passing a function. Since the menu is not really a React component, it might not constantly re-rendered. Not sure how we want to handle this. (This issues is present in a few of our Dropdowns.)
Also the autoDestroy prop is also not mentioned in the docs anymore. There is a reference to destroyPopupOnHide though, which sounds similar: https://4x.ant.design/components/dropdown/#API

@hotzenklotz hotzenklotz self-assigned this Mar 13, 2023
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Avoid SQL error when fetching view config for zero-layer dataset (#6912)
  Fix date formatting for VX reports (#6908)
  Fix rare crash when viewing shared annotation (#6892)
  Slim down view mode dropdown by using icons (#6900)
  Logging on password reset/change (#6901)
  When merging volume tracings, also merge segment lists (#6882)
  avoid spinner when switching tabs in dashboard (#6894)
@hotzenklotz hotzenklotz requested a review from philippotto March 13, 2023 14:07
@hotzenklotz hotzenklotz marked this pull request as ready for review March 13, 2023 14:07
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! didn't test yet, but have already left some small comments.

CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
@philippotto
Copy link
Member

philippotto commented Mar 15, 2023

First round of testing:

  • right clicking a dataset in the dashboard and hitting escape (or clicking somewhere else) crashes the dashboard
  • right clicking a skeleton node shows a position icon which is different from the icon used in the position box. I think, using the same icon would be better
  • right clicking a position in a dataset, doesn't show a position icon for me (however, there's an icon in code, I think)

@philippotto
Copy link
Member

philippotto commented Mar 16, 2023

One other problem:

  • the MenuItemWithMappingActivationConfirmation component doesn't seem to work anymore (it doesn't do anything regardless of the selected mapping).

Steps to reproduce:

@philippotto
Copy link
Member

I also just tested performance and everything seemed fine 👍 (for the record, I tested with 80k trees).

…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Fix download button for annotations when tiff export is disabled (#6931)
  Update PULL_REQUEST_TEMPLATE.md
  Prepare multi modality support (#6748)
  Improvements for terms-of-services modal (#6930)
  Fix creating task types with preferred mode (#6928)
  Fix styling for login pages for dark mode users (#6916)
  Added members to admin team page (#6915)
  Release 23.03.1 (#6917)
  Redesign Welcome UI (#6904)
  Fix race condition which could disable saving (#6914)
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Update screenshots (#6934)
  Support rendering negative floats (#6895)
  Fix loading of webworkers in dev mode (#6933)
  Restore cache buster for webworkers (#6932)
  Introduce data vault as storage backend abstraction (#6899)
@hotzenklotz
Copy link
Member Author

@philippotto I think I fixed the issues that you detected and applied your PR feedback. Please have a look at my last few commits.

I likely also fixed issue #6814 in the process.

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splendid clean up! See only my one comment about commented code before merging.

frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Add email notifications for processing jobs (#6918)
  Send Cache-Control: no-cache for most routes (#6937)
@hotzenklotz hotzenklotz enabled auto-merge (squash) March 23, 2023 12:39
@hotzenklotz hotzenklotz merged commit 8c79390 into master Mar 23, 2023
@hotzenklotz hotzenklotz deleted the dropdown-menu branch March 23, 2023 13:36
hotzenklotz added a commit that referenced this pull request Apr 3, 2023
…come-toast

* 'master' of github.com:scalableminds/webknossos:
  Log all details on deleting annotation layer (#6950)
  fix typo
  Rename demo instance to wkorg instance (#6941)
  Add LOD mesh support for frontend (#6909)
  Fix layout of view mode switcher and move it (#6949)
  VaultPath no longer extends nio.Path (#6942)
  Release 23.04.0 (#6945)
  Use new zip.js version to allow zip64 uploads (#6939)
  Implement viewing sharded neuroglancer precomputed datasets (#6920)
  Reject dataset uploads if organization storage quota is exceeded (#6893)
  Refactor deprecated antd Dropdown menus (#6898)
hotzenklotz added a commit that referenced this pull request Apr 4, 2023
…wings

* 'master' of github.com:scalableminds/webknossos:
  updates docs for docker installation (#6963)
  Fix misc stuff when viewing tasks/annotations of another user (#6957)
  Remove segment from list and add undo/redo for segments (#6944)
  Log all details on deleting annotation layer (#6950)
  fix typo
  Rename demo instance to wkorg instance (#6941)
  Add LOD mesh support for frontend (#6909)
  Fix layout of view mode switcher and move it (#6949)
  VaultPath no longer extends nio.Path (#6942)
  Release 23.04.0 (#6945)
  Use new zip.js version to allow zip64 uploads (#6939)
  Implement viewing sharded neuroglancer precomputed datasets (#6920)
  Reject dataset uploads if organization storage quota is exceeded (#6893)
  Refactor deprecated antd Dropdown menus (#6898)
@hotzenklotz hotzenklotz mentioned this pull request Jul 10, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent call order of hooks in context menu
2 participants